fix: add LAN disable and SD stream interface setup to StartSdCardLoggingAsync#153
Merged
fix: add LAN disable and SD stream interface setup to StartSdCardLoggingAsync#153
Conversation
…ingAsync SD card and LAN share the SPI bus on the hardware. StartSdCardLoggingAsync was missing the DisableNetworkLan and SetStreamInterface(SdCard) commands that the desktop app was sending via its PrepareSdInterface() workaround. This caused SD card logging to silently produce no files when called from Core alone (e.g. via the example CLI app). StopSdCardLoggingAsync now also restores the stream interface to USB for USB connections so subsequent non-SD operations work correctly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: a0fb8798807b
Review Summary by QodoFix SD card logging by disabling LAN and configuring stream interface
WalkthroughsDescription• Disable LAN and set stream interface to SD card before logging • Restore stream interface to USB after SD card logging stops • Update tests to verify new 6-command sequence instead of 4 Diagramflowchart LR
A["StartSdCardLoggingAsync"] --> B["DisableNetworkLan"]
B --> C["EnableStorageSd"]
C --> D["SetStreamInterface SdCard"]
D --> E["SetSdLoggingFileName"]
E --> F["SetStreamFormat"]
F --> G["StartStreamData"]
H["StopSdCardLoggingAsync"] --> I["StopStreaming"]
I --> J["DisableStorageSd"]
J --> K["SetStreamInterface USB"]
File Changes1. src/Daqifi.Core/Device/DaqifiStreamingDevice.cs
|
Code Review by Qodo
1.
|
4 tasks
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
448
to
+456
| StopStreaming(); | ||
| Send(ScpiMessageProducer.DisableStorageSd); | ||
|
|
||
| // Restore stream interface to USB so subsequent non-SD operations work. | ||
| if (IsUsbConnection) | ||
| { | ||
| Send(ScpiMessageProducer.SetStreamInterface(StreamInterface.Usb)); | ||
| } | ||
|
|
There was a problem hiding this comment.
2. Lan not restored on stop 🐞 Bug ⛯ Reliability
StopSdCardLoggingAsync disables SD storage but never re-enables LAN after StartSdCardLoggingAsync disabled it, leaving the device in a LAN-disabled state after logging. This can break later network operations unless callers know to manually call PrepareLanInterface.
Agent Prompt
### Issue description
`StopSdCardLoggingAsync` does not restore the LAN interface after `StartSdCardLoggingAsync` disables it, leaving the device in a LAN-disabled state.
### Issue Context
The class already defines `PrepareLanInterface()` as the standard way to restore LAN (it disables SD storage and enables LAN). Other SD operations restore LAN in `finally` blocks.
### Fix Focus Areas
- src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[439-460]
- src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[272-285]
### Suggested fix
- In `StopSdCardLoggingAsync`, after `Send(DisableStorageSd)`, restore LAN using one of:
- Call `PrepareLanInterface()` (may resend DisableStorageSd but is idempotent), or
- Send `ScpiMessageProducer.EnableNetworkLan` explicitly.
- Consider adding a short settle delay (reuse `SD_INTERFACE_SETTLE_DELAY_MS`) if subsequent commands are likely to follow immediately.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
SD card logging requires USB because SD and LAN share the SPI bus. Add an early guard that throws InvalidOperationException for non-USB connections, matching the existing pattern in DownloadSdCardFileAsync. Also fixes StopSdCardLoggingAsync test to expect the new SetStreamInterface(Usb) restore command, and updates the download non-USB test to use a dedicated TestableNonUsbStreamingDevice. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: a0fb8798807b
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
StartSdCardLoggingAsyncnow sendsDisableNetworkLanandSetStreamInterface(SdCard)before enabling SD storage and starting streaming. SD card and LAN share the SPI bus on the hardware — without disabling LAN first, SD card logging silently produces no files.StopSdCardLoggingAsyncnow restores the stream interface to USB (for USB connections) so subsequent non-SD operations work correctly.Context
The desktop app had been working around this via its
PrepareSdInterface()method, which sent these commands before calling Core. When using Core directly (e.g. via the CLI example app), SD card logging would appear to start/stop successfully but no files were written to the card.Test plan
--sd-log-start --sd-log-format csvvia example CLI creates a file on the SD card--sd-log-start --sd-log-format protobufvia example CLI creates a file on the SD card--sd-listshows the new files after logging🤖 Generated with Claude Code